Hotfix/9.2.11 by jasonleenaylor · Pull Request #465 · sillsdev/FieldWorks

jasonleenaylor · 2025-09-19T16:26:40Z

This change is

* Reuse wordforms that are matches but create new anaysis for them * Add Normalization before string comparison * Split Morpheme and glossing logic Goal: If the imported data is a subset of an existing analysis then use the existing analysis, otherwise create a new analysis and add it to a matching Wordform. # Conflicts: # Src/LexText/Interlinear/BIRDInterlinearImporter.cs

* Fix LT-22267: Slow response in interlinear combos * Remove unused using # cherry-picked from c531282

GenerateCssFromWsOptions() was only generating rules for one writing system. Changed this to generate rules for all enabled writing systems.

Copilot

Pull Request Overview

This PR refactors the BIRD format interlinear import logic to improve matching of existing wordforms and analyses, reducing duplicate creation. It also includes minor bug fixes and updates to build configuration.

Refactored CreateWordAnalysisStack into CreateWordformWithWfiAnalysis with improved matching logic that checks morphemes and glosses
Fixed GenerateCssFromWsOptions to accumulate all rules instead of returning on first match
Updated environment variable names in build tasks from Jenkins to GitHub Actions conventions

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
Src/xWorks/CssGenerator.cs	Fixed early return bug by accumulating CSS rules for all writing systems
Src/MasterVersionInfo.txt	Version bump from 9.2.10 to 9.2.11
Src/LexText/Interlinear/WordsSfmImportWizard.cs	Added mainWs parameter to ImportWordsFrag call
Src/LexText/Interlinear/LinguaLinksImport.cs	Updated method signatures to accept mainWs parameter and renamed method call
Src/LexText/Interlinear/ITextDllTests/ImportInterlinearAnalysesTests.cs	Added comprehensive test coverage for morpheme matching and improved variable naming
Src/LexText/Interlinear/ITextDllTests/BIRDFormatImportTests.cs	Removed ignored test case
Src/LexText/Interlinear/ChooseAnalysisHandler.cs	Simplified gloss retrieval by removing guess services and reordered using statements
Src/LexText/Interlinear/BIRDInterlinearImporter.cs	Major refactoring of analysis matching logic with new helper methods
Build/nuget-common/packages.config	Added System.Buffers package dependency
Build/mkall.targets	Added System.Buffers.dll to build configuration
Build/Src/FwBuildTasks/Substitute.cs	Updated environment variable names for GitHub Actions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-03T18:52:30Z

 			LCModel.IText text;

-			IWfiWordform word = null;
+			IWfiWordform extandWordForm = null;


Corrected spelling of 'extand' to 'extant'.

Copilot · 2025-11-03T18:52:30Z

-				var newWfiAnalysis = sl.GetInstance<IWfiAnalysisFactory>().Create();
-				word.AnalysesOC.Add(newWfiAnalysis);
-				segment.AnalysesRS.Add(word);
+				extandWordForm = sl.GetInstance<IWfiWordformFactory>().Create(wform);


Corrected spelling of 'extand' to 'extant'.

Copilot · 2025-11-03T18:52:30Z

-				segment.AnalysesRS.Add(word);
+				extandWordForm = sl.GetInstance<IWfiWordformFactory>().Create(wform);
+				var extantAnalysis = sl.GetInstance<IWfiAnalysisFactory>().Create();
+				extandWordForm.AnalysesOC.Add(extantAnalysis);


Corrected spelling of 'extand' to 'extant'.

Copilot · 2025-11-03T18:52:31Z

+				extandWordForm = sl.GetInstance<IWfiWordformFactory>().Create(wform);
+				var extantAnalysis = sl.GetInstance<IWfiAnalysisFactory>().Create();
+				extandWordForm.AnalysesOC.Add(extantAnalysis);
+				segment.AnalysesRS.Add(extandWordForm);


Corrected spelling of 'extand' to 'extant'.

Copilot · 2025-11-03T18:52:31Z

 			foreach (var wordItem in word.Items)
 			{
+				if (wordItem.Value == null)
+					continue;
+
+				var ws = GetWsEngine(wsFact, wordItem.lang);
+
 				switch (wordItem.type)
 				{
 					case "txt":
-						var wsAlt = GetWsEngine(wsFact, wordItem.lang);
-						if (wsAlt.Handle == wsMainVernWs.Handle)
+						wordForm = TsStringUtils.MakeString(wordItem.Value, ws.Handle);
+						expectedForms[ws.Handle] = wordItem.Value;
+
+						// Try to find a candidate wordform if we haven't found one yet
+						if (candidateForm == null)
+						{
+							candidateForm = cache.ServiceLocator
+								.GetInstance<IWfiWordformRepository>()
+								.GetMatchingWordform(ws.Handle, wordItem.Value);
+						}
+
+						break;
+
+					case "punct":
+						punctForm = TsStringUtils.MakeString(wordItem.Value, ws.Handle);
+						expectedForms[ws.Handle] = wordItem.Value;
+
+						if (candidateForm == null)
+						{
+							IPunctuationForm pf;
+							if (cache.ServiceLocator.GetInstance<IPunctuationFormRepository>()
+								.TryGetObject(punctForm, out pf))
+							{
+								candidateForm = pf;
+							}
+						}
+
+						break;
+
+					case "gls":
+						// Only consider human-approved glosses
+						if (wordItem.analysisStatusSpecified &&
+							wordItem.analysisStatus != analysisStatusTypes.humanApproved)
 							continue;
-						ITsString wffAlt = TsStringUtils.MakeString(wordItem.Value, wsAlt.Handle);
-						if (wffAlt.Length > 0)
-							wf.Form.set_String(wsAlt.Handle, wffAlt);
+
+						expectedGlosses[ws.Handle] = wordItem.Value;
 						break;
 				}
 			}


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2025-11-03T18:52:31Z

 						itemDict[item.type] = new Tuple<string, string>(item.lang, item.Value);
-					}

 					if (itemDict.ContainsKey("txt")) // Morphemes


Inefficient use of 'ContainsKey' and indexer.
Inefficient use of 'ContainsKey' and indexer.

Copilot · 2025-11-03T18:52:32Z

+				"New MorphBundles should not have been created.");
+
+			// Verify the imported analysis is the same object
+			var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;


Variable importedPara may be null at this access because of this assignment.

Suggested change

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

Assert.NotNull(importedPara, "importedPara should not be null. Check that ParagraphsOS[0] is an IStTxtPara.");

Copilot · 2025-11-03T18:52:32Z

+				"New MorphBundles should not have been created.");
+
+			// Verify the imported analysis is the same object
+			var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;


Variable importedPara may be null at this access because of this assignment.

Suggested change

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

Assert.That(importedPara, Is.Not.Null, "ParagraphsOS[0] is not an IStTxtPara; importedPara is null.");

Copilot · 2025-11-03T18:52:33Z

+				"Two new MorphBundles should have been created.");
+
+			// Verify the imported analysis and its contents
+			var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;


Variable importedPara may be null at this access because of this assignment.

Suggested change

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;

if (importedPara == null)

Assert.Fail("ParagraphsOS[0] is not an IStTxtPara as expected.");

Copilot · 2025-11-03T18:52:33Z

+
+			// Verify the imported analysis and its contents
+			var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;
+			if(!(importedPara.SegmentsOS[0].AnalysesRS[0] is IWfiAnalysis importedAnalysis))


Variable importedPara may be null at this access because of this assignment.

Suggested change

if(!(importedPara.SegmentsOS[0].AnalysesRS[0] is IWfiAnalysis importedAnalysis))

if (importedPara == null)

Assert.Fail("ParagraphsOS[0] is not an IStTxtPara");

else if (!(importedPara.SegmentsOS[0].AnalysesRS[0] is IWfiAnalysis importedAnalysis))

jasonleenaylor and others added 6 commits September 19, 2025 08:58

Bump version to 9.2.11

8b8b050

Fix LT-22267: Slow response in interlinear combos (#445)

db3ab36

* Fix LT-22267: Slow response in interlinear combos * Remove unused using # cherry-picked from c531282

LT-22185: XHTML: Fix style when there are multiple ws (#454)

5a1e101

GenerateCssFromWsOptions() was only generating rules for one writing system. Changed this to generate rules for all enabled writing systems.

Pin System.Buffers dependency to nuget package serving 4.0.3

e44fc32

Fix the assembly stamping of the base build after moving to gha

50bdcd9

jasonleenaylor marked this pull request as ready for review November 3, 2025 18:42

Copilot AI review requested due to automatic review settings November 3, 2025 18:42

jasonleenaylor enabled auto-merge November 3, 2025 18:44

jasonleenaylor disabled auto-merge November 3, 2025 18:47

jasonleenaylor merged commit c0209df into master Nov 3, 2025
8 checks passed

jasonleenaylor deleted the hotfix/9.2.11 branch November 3, 2025 18:47

Copilot AI reviewed Nov 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Hotfix/9.2.11#465

Hotfix/9.2.11#465
jasonleenaylor merged 6 commits into
masterfrom
hotfix/9.2.11

jasonleenaylor commented Sep 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Copilot AI Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;
	var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;
	Assert.NotNull(importedPara, "importedPara should not be null. Check that ParagraphsOS[0] is an IStTxtPara.");

	var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;
	var importedPara = importedText.ContentsOA.ParagraphsOS[0] as IStTxtPara;
	Assert.That(importedPara, Is.Not.Null, "ParagraphsOS[0] is not an IStTxtPara; importedPara is null.");

-			if(!(importedPara.SegmentsOS[0].AnalysesRS[0] is IWfiAnalysis importedAnalysis))
+			if (importedPara == null)
+				Assert.Fail("ParagraphsOS[0] is not an IStTxtPara");
+			else if (!(importedPara.SegmentsOS[0].AnalysesRS[0] is IWfiAnalysis importedAnalysis))

Uh oh!

Uh oh!

Conversation

jasonleenaylor commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jasonleenaylor commented Sep 19, 2025 •

edited

Loading